Skip to content

Conversation

@d-w-moore
Copy link
Collaborator

In other words, setting opt2 = val2 does not reset opt1 back to its default value.

Copy link
Contributor

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.

@d-w-moore d-w-moore changed the title [_709] preserve options in chaining obj.metadata(opt1=val1)(opt2=val2) [#709] preserve options in chaining obj.metadata(opt1=val1)(opt2=val2) May 6, 2025
@d-w-moore d-w-moore marked this pull request as draft November 27, 2025 04:44
@d-w-moore d-w-moore force-pushed the 709.m branch 2 times, most recently from ef597fa to 087ded2 Compare November 29, 2025 03:24
@d-w-moore d-w-moore self-assigned this Nov 29, 2025
@d-w-moore
Copy link
Collaborator Author

Evidently it is still possible to fail here:

https://github.com/d-w-moore/python-irodsclient/blob/388ad3d858ebcc7be981d1c065a4dd57ffc066cf/irods/test/data_obj_test.py#L3317

The modify time turned out on this occasion to be one second later than the access time on a just-created replica open for write:


======================================================================
FAIL [0.216s]: test_access_time__issue_700 (data_obj_test.TestDataObjOps.test_access_time__issue_700)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/prc.rw/irods/test/data_obj_test.py", line 3317, in test_access_time__issue_700
    self.assertEqual(data.access_time, data.modify_time)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: datetime.datetime(2025, 11, 29, 4, 12, 23, tzinfo=datetime.timezone.utc) != datetime.datetime(2025, 11, 29, 4, 12, 24, tzinfo=datetime.timezone.utc)

----------------------------------------------------------------------
Ran 302 tests in 319.841s

FAILED (failures=1, skipped=16)

@korydraughn any ideas why this might occasionally happen?

@d-w-moore d-w-moore marked this pull request as ready for review November 29, 2025 04:31
@d-w-moore
Copy link
Collaborator Author

Squashed , ready for review.

Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am especially interested in the changes related to #768 given the milestone for that issue.

@korydraughn
Copy link
Contributor

korydraughn commented Dec 1, 2025

Evidently it is still possible to fail here:

https://github.com/d-w-moore/python-irodsclient/blob/388ad3d858ebcc7be981d1c065a4dd57ffc066cf/irods/test/data_obj_test.py#L3317

The modify time turned out on this occasion to be one second later than the access time on a just-created replica open for write:


======================================================================
FAIL [0.216s]: test_access_time__issue_700 (data_obj_test.TestDataObjOps.test_access_time__issue_700)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/prc.rw/irods/test/data_obj_test.py", line 3317, in test_access_time__issue_700
    self.assertEqual(data.access_time, data.modify_time)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: datetime.datetime(2025, 11, 29, 4, 12, 23, tzinfo=datetime.timezone.utc) != datetime.datetime(2025, 11, 29, 4, 12, 24, tzinfo=datetime.timezone.utc)

----------------------------------------------------------------------
Ran 302 tests in 319.841s

FAILED (failures=1, skipped=16)

@korydraughn any ideas why this might occasionally happen.

Hmm, I don't think the python test code is equivalent to the following C++ test code?

https://github.com/irods/irods/blob/1a5670d157ce932cc2888d74bb29c831e2db53ae/unit_tests/src/test_rc_update_replica_access_time.cpp#L312-L323

https://github.com/irods/irods/blob/1a5670d157ce932cc2888d74bb29c831e2db53ae/unit_tests/src/test_rc_update_replica_access_time.cpp#L689-L706

The C++ test code I linked to expects the replica to be in the intermediate state.

I think the problem with the python test is that the following call closes the data object, resulting in the close operation updating the mtime.

data = self.sess.data_objects.create(

What you need to do is open (and create) a new replica and do your checks while the replica is open. Closing the replica is what causes the atime and mtime to desync.

@d-w-moore
Copy link
Collaborator Author

What you need to do is open (and create) a new replica and do your checks while the replica is open. Closing the replica is what causes the atime and mtime to desync.

Ah, you're right. I see. Will try it a different way.

@korydraughn
Copy link
Contributor

What's the status of this PR?

@d-w-moore
Copy link
Collaborator Author

What's the status of this PR?

Looks like we are good. Need a squash, and perhaps a codacy review.

Copy link
Contributor

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.

Just a few comments on the README.

@d-w-moore d-w-moore force-pushed the 709.m branch 3 times, most recently from b673bdc to dfb0cd9 Compare December 8, 2025 20:16
@d-w-moore
Copy link
Collaborator Author

d-w-moore commented Jan 28, 2026

To this subject
We can now , in this PR, do the following :

d_bin_meta = d.metadata(iRODSMeta_type = iRODSBinOrStringMeta)
d_bin_meta.set ( 'key1', b'arbitrary-octets-\xe1\x80')

And later: d_bin_meta ['key1'] (or an object similarly conditioned, in another application perhaps ) automatically returns an iRODSBinOrStringMeta('key1',b'arbitrary-octets-\xe1\x80') AVU

At worst, it's the client lib doing too much.

At best, it smooths a path that Python2 folks hypothetically might have relied on in the past (since bytestrings and strings were the same back then).

Copy link
Contributor

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ready to me.

Squash to taste once the review comment is resolved.

@korydraughn
Copy link
Contributor

I see a revert in the git history?
Was that planned/expected?

What's the status of this PR?

@d-w-moore
Copy link
Collaborator Author

I see a revert in the git history?
Was that planned/expected?

The revert was merely to remove an unrelated change, a commit that shouldn't have made it in....

…val2)

In other words, setting opt2 = val2 does not reset opt1 back to its default value.
@d-w-moore
Copy link
Collaborator Author

Squashed just now.

@korydraughn
Copy link
Contributor

Okay. Please resolve the ruff-check/format reports.

The "Files Changed" page (diff) should highlight the lines reported by the ruff workflows. You can use that view with the github actions report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants